fix(sheets): align +workbook-create, +dropdown-*, +dim-move, +range-sort with server schema (restore --colors/--highlight)#1030
Merged
zhengzhijiej-tech merged 11 commits intoMay 25, 2026
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
c0b1ffa to
370137e
Compare
514b5e1 to
538eb2e
Compare
…ort with server schema
Five separate E2E failures in shortcuts/sheets/ that all trace back to a
CLI ↔ server contract mismatch. Each is independently scoped; bundling
them because they share the test-report citation and the same one-line
fix shape in most cases.
buildInitialFillInput sent {"sheet_id": ""} on the secondary
set_cell_range call after creating the workbook. The empty value was a
holdover from "...otherwise server picks first sheet" — but
set_cell_range rejects an empty selector with
"sheet_id or sheet_name is required" rather than falling back to the
default sheet.
Use sheet_name "Sheet1" instead. POST /sheets/v3/spreadsheets always
creates that sheet on workbook creation, and set_cell_range accepts
sheet_name as an equivalent selector — saves an extra
get_workbook_structure round-trip just to learn the auto-generated id.
buildDropdownValidation emitted four fields that don't exist in the
canonical set_cell_range.data_validation schema:
- "values" (options list) → renamed to "items"
- "multiple_values" → renamed to "support_multiple_values"
- "colors" (per-option color) → removed (not in schema; flag also
removed from data/flag-defs.json
for +dropdown-set / -update)
- "highlight_options" → removed (not in schema; flag also
removed)
The canonical schema lives at sheet-skill-spec/canonical-spec/tool-
schemas/mcp-tools.json (set_cell_range tool, data_validation property);
the colors / highlight knobs were CLI inventions the server never
accepted, so removing the flags is correct (renaming would leave the
flags broken). Skill reference docs (write-cells.md, batch-update.md)
synced.
validateDropdownOptionsColors lost its colors check; renamed to
validateDropdownOptions to reflect the narrower contract.
dropdownGetInput sent "Sheet1!C2:C6" verbatim as a ranges[] entry.
get_cell_ranges expects sheet_id / sheet_name as separate fields and
ranges entries without the sheet prefix; the server bounced with
"sheet not found, sheetId:" (empty).
Use the existing splitSheetPrefixedRange helper (declared in
lark_sheet_batch_update.go) to break "Sheet1!C2:C6" into ("Sheet1",
"C2:C6"), then thread the sheet name through sheetSelectorForToolInput
exactly like +cells-get does.
The shortcut was POSTing to /sheets/v2/spreadsheets/{token}/dimension_
range, which is the v2 insert-dimension endpoint and requires a top-
level {"dimension": {...}} body. Move uses a separate endpoint:
POST /sheets/v2/spreadsheets/{token}/move_dimension
body: { "source": {...}, "destination_index": N }
(camelCase "destinationIndex" → snake_case "destination_index" to
match the v2 contract.) Both DryRun and Execute updated, plus the
TestDimMove_DryRun and TestExecute_DimMove assertions.
transform_range.sort_conditions[i] requires both `column` (string) and
`ascending` (bool); rangeSortInput passed the --sort-keys array through
to the server unvalidated, so missing fields surfaced as opaque
"required property X missing" errors with no per-item context.
Walk the parsed array client-side, reject with item-pointing messages.
Test fixtures and a contract-test fixture switched from the historical
{col, order} vocabulary (which the server has never accepted) to the
correct {column, ascending}.
Server-schema citations and test-report case mapping in this branch's
plan file.
data/flag-defs.json is regenerated from the upstream sheet-skill-spec
canonical-spec; editing it here gets clobbered on the next sync. The
schema realignment for +dropdown-set / -update --colors / --highlight
removal needs to land on the base table first, then flow back through
sheet-skill-spec → larksuite-cli sync, not via a direct CLI-side edit.
Restore the previous flag entries verbatim. The Go-side change in
buildDropdownValidation still drops the wire fields, so:
- users passing --colors / --highlight today see the flag accepted
silently (no effect on the wire) until the upstream removal lands;
- after upstream removal + sync, both the flag declarations and the
Go-side handling will be in sync.
Functional fixes (larksuite#1 workbook-create, larksuite#3 dropdown-get, larksuite#4 dim-move,
larksuite#5 range-sort) and dropdown wire-shape rename (larksuite#2) are unaffected.
These md files are sync targets generated from sheet-skill-spec; editing
them here gets clobbered on the next sync, same as data/flag-defs.json.
The --colors / --highlight row removals belong on the upstream base
table → canonical-spec sync, not here.
Restore the previous --colors / --highlight rows in both
lark-sheets-write-cells.md (+dropdown-set) and lark-sheets-batch-update.md
(+dropdown-update). The Go-side change in buildDropdownValidation still
drops the wire fields, so:
- users passing --colors / --highlight today see the flag accepted
silently (no effect on the wire) until upstream removes the flag;
- after upstream removal + sync, both flag declarations, ref docs, and
Go-side handling will be in sync.
Functional fixes (larksuite#1 workbook-create, larksuite#3 dropdown-get, larksuite#4 dim-move,
larksuite#5 range-sort) and dropdown wire-shape rename (larksuite#2) are unaffected.
… --highlight Upstream sheet-skill-spec base table deleted the --colors and --highlight flags on +dropdown-set / +dropdown-update (the corresponding wire fields data_validation.colors / .highlight_options were never accepted by the server schema; see prior fix in this branch). Re-running the sync from canonical-spec brings the CLI flag-defs and skill reference docs back in line with the Go-side handling that already drops these fields. Generated by `npm run sync:cli` in sheet-skill-spec @ ac7acef.
…al fields Reverses the --colors / --highlight removal from 7932ab2 (item larksuite#2 of the batch-1 schema-alignment commit). That commit dropped both flags after the test report flagged data_validation.colors / highlight_options as "unexpected property" — at the time the canonical set_cell_range.data_validation schema listed only help_text / items / operator / range / support_multiple_values / type / values, so the flags had no server-side target and the removal was correct. Since then, set_cell_range.data_validation has gained two fields explicitly modelling the dropdown highlight UI (mcp-tools.json in sheet-skill-spec 2026-05-22 base sync): enable_highlight (bool) — show pill backgrounds highlight_colors (string[]) — hex pill colors, length must match items So the flags are back, but rewired: --colors -> data_validation.highlight_colors (was: colors) --highlight -> data_validation.enable_highlight (was: highlight_options) --options -> items and --multiple -> support_multiple_values renames from 7932ab2 are kept. Changes: - buildDropdownValidation: re-add --colors / --highlight handling against the new field names; --colors length check stays inline (so dropdownSetInput Validate path catches it via validateViaInput, no separate guard needed). - validateDropdownOptions -> validateDropdownOptionsColors: restore the Validate-time --colors length check on +dropdown-update / +dropdown-delete (called from lark_sheet_batch_update.go). - TestDropdownSet_CellsShape: extend to assert highlight_colors / enable_highlight emitted; assert legacy `colors` / `highlight_options` absent. - TestDropdownSet_ColorsLengthMismatch: new — covers the early Validate error path. - TestDropdownUpdate_BatchPayload: extend to cover dropdownBatchInput propagation of --colors / --highlight through batch_update. - skills/lark-sheets/references/lark-sheets-{write-cells,batch-update}.md, shortcuts/sheets/data/flag-defs.json, flag-schemas.json: synced from sheet-skill-spec generate output (MR !7).
Catches up to sheet-skill-spec's 2026-05-25 base sync (MR !7) after
rebasing onto upstream feat/lark-sheets-refactor (12 new upstream commits
including the lark-sheets skill refactor + tools-schema migration).
Spec changes flowing in:
- highlight_colors description loosened: length may be **shorter than**
--options (server cycles remaining slots through a built-in 10-color
palette); previously the tool errored on any length mismatch.
- shortcuts/sheets/data/flag-schemas.json: mass re-mirror — generator now
emits `type` before `properties` and adds explicit `additionalProperties:
false` on object schemas (cosmetic, no behavior change).
- skills/lark-sheets/references/lark-sheets-{batch-update,chart,write-cells}.md:
--options gains the type='list' tag; data_validation inline field-count
goes 7 → 9 (catches up the highlight schema in the summary); chart
position / size marked optional per upstream.
Go-side adjustment:
- buildDropdownValidation / validateDropdownOptionsColors: change the
--colors length check from strict-equal to "must not exceed --options"
to match the relaxed schema.
- TestDropdownSet_ColorsLengthMismatch -> TestDropdownSet_ColorsLongerThanOptions
(now hits the overflow path with 3 colors vs 2 options).
- New TestDropdownSet_ColorsShorterAccepted: 2 colors vs 4 options is
legal and forwarded as-is.
Mirrors sheet-skill-spec MR !7 changes: - skills/lark-sheets/references/lark-sheets-write-cells.md: new "Dropdown 配色" section explaining how --colors (→ data_validation.highlight_colors) and --highlight (→ data_validation.enable_highlight) compose — length rule (shorter ok, longer rejected), --highlight gating, palette fallback behavior, minimal +dropdown-set example. - skills/lark-sheets/references/lark-sheets-batch-update.md: one-line pointer to the write_cells section for +dropdown-update / -delete (same rules). - shortcuts/sheets/data/flag-defs.json: --colors / --highlight `desc` fields gain the long-form server-field / length-rule descriptions used by `--help`. No Go-side change — earlier commit 538eb2e already loosened the buildDropdownValidation length check to "must not exceed"; this PR step just makes the docs / `--help` text catch up.
…mode
Previously +dropdown-set / +dropdown-update only emitted
data_validation.type=list — agents wanting listFromRange (dropdown options
sourced from existing cells, kept in sync with that range) had to drop down
to +cells-set and hand-build a data_validation map. The flag now exposes it
natively as --source-range, paired with --options under XOR.
CLI changes:
- shortcuts/sheets/lark_sheet_write_cells.go:
* new dropdownTypeAndItems(runtime) — central XOR resolver: rejects 0 or
2 of {--options, --source-range}, returns (sourceSize, partial dv with
type+items|range filled in). Source size = options length for list
mode, rangeDimensions(--source-range) cell count for listFromRange.
* buildDropdownValidation rewritten to call the resolver, then layer
--colors / --multiple / --highlight on top — semantics unchanged
for callers, just two modes instead of one.
* validateDropdownOptions / -Colors renamed to validateDropdownSourceOrOptions
so the XOR + length check fires at +dropdown-update Validate time too.
* --colors length error message generalized: "must not exceed dropdown
source size (N)" (covers both modes).
- shortcuts/sheets/lark_sheet_batch_update.go: rename call site.
- shortcuts/sheets/lark_sheet_write_cells_test.go: 4 new tests —
ListFromRange (happy path: range + items absent + colors + highlight all
emit), ListFromRange_ColorsLongerThanCells (overflow against T1:T3 cell
count), XorBothSet, XorNeitherSet. Updated the existing
ColorsLongerThanOptions assertion to match the new "source size" wording.
Spec-driven changes (synced via npm run sync:cli from sheet-skill-spec
MR !7 2c298b6):
- shortcuts/sheets/data/flag-defs.json: --options Required flips to xor on
+dropdown-set/-update; new --source-range row gains long-form description
pointing at server data_validation.range + the XOR semantics.
- skills/lark-sheets/references/lark-sheets-write-cells.md: "Dropdown 配色"
section reorganized into "Dropdown 选项 + 配色" — XOR comparison table
(list vs listFromRange), shared config flag table (--highlight /
--colors), explicit length rule covering both modes, side-by-side
minimal examples, server-range-normalization gotcha callout.
- skills/lark-sheets/references/lark-sheets-batch-update.md pointer updated
to mention both modes + that +dropdown-delete is unaffected.
PPE smoke (ppe_lark_cli_sheet) on UFJxszjrZhZ1LVtc9FdcICSbn6b C column:
- +cells-set C1 → "性别" (bold + centered): updated_cells_count=1
- +dropdown-set --range C2:C21 --source-range "Sheet1!T1:T3" --colors
'["#cce8ff","#ffd6e7","#e6e6e6"]' --highlight: updated_cells_count=20
- read-back: data_validation.type=listFromRange + range=$T$1:$T$3 (server
normalizes the prefix away on storage; highlight_colors /
enable_highlight not echoed by get_cell_ranges, see byted-sheet read
projection TODO).
- error-path replay (both XOR violations + colors > source-size) all
rejected at Validate stage with the expected messages.
Mirrors sheet-skill-spec MR !7 60df610 — narrative now describes how the flags interact (XOR, colors length rule, highlight gating, sheet-prefix read-back gotcha) without exposing the underlying data_validation field names or server-side normalization details that agents don't act on. No Go-side change, no shortcut behavior change.
The earlier commit 49104ec swapped --colors out of parseJSONFlag's "Used by" example list when it deleted the flag (item larksuite#2 there removed --colors / --highlight from +dropdown-set/-update). Subsequent commits 8672d8e / 538eb2e / fb90c8b reinstated --colors (and added --source-range) but did not roll back this docstring tweak — leaving an orphan reference to --properties where --colors used to be. This restores the example list to its pre-49104ec form so the docstring matches what the helper actually services on this branch's HEAD. Pure docstring change — function behavior unaffected, no test movement.
…rksuite#1 Two test fallouts from rebasing onto upstream 4be06c8 (which independently re-fixed +workbook-create and +dim-move with a more thorough approach): - shortcuts/sheets/lark_sheet_workbook_test.go: our PR's earlier TestWorkbookCreate_DryRun "with headers and data → 2-step plan" subtest asserted the expedient sheet_name="Sheet1" / no-sheet_id wire body that matched our dropped fix larksuite#1 implementation. Upstream's fix larksuite#1 resolves the workbook's first sheet via get_workbook_structure and fills with the real sheet_id instead. Reset this file to upstream's version — our superseded assertions disappear, upstream's tests cover the new wire shape. - shortcuts/sheets/execute_paths_test.go: TestExecute_RangeSort fixture still used the legacy {col, order} sort-key shape because the rebase resolution picked the upstream version of this file wholesale (it contained other unrelated changes). Re-apply just the one fixture update to {column, ascending} so fix larksuite#5's CLI-side rejection logic exercises a valid input — server-side sort_conditions has required fields `column` (string) and `ascending` (bool); the historical {col, order} vocabulary was never accepted. go build ./... + go test ./shortcuts/sheets/... -count=1 both green.
35bf25e to
f0dea38
Compare
5eaa70b
into
larksuite:feat/lark-sheets-refactor
3 checks passed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Six independent schema-alignment fixes for E2E test report cases on this branch
(commit
1e05e7b, PPE laneppe_lark_cli_sheet). Each matches a specificserver error that traces back to a CLI ↔ canonical-schema mismatch.
+workbook-create --headers/--valuesset_cell_range: sheet_id or sheet_name is requiredbuildInitialFillInputsent{"sheet_id":""}(assumed server-side default fallback that doesn't exist)+dropdown-set / -updatedata_validation.{colors,multiple_values,highlight_options} unexpected propertyset_cell_range.data_validation+dropdown-getsheet not found, sheetId:(empty)dropdownGetInputsentSheet1!C2:C6verbatim instead of splitting into sheet_name + bare A1+dim-move[9499] Missing required parameter: Dimension/dimension_range= insert) + camelCase body key+range-sortrequired property "column"/"ascending" missing--sort-keysitems passed through unvalidated; field-naming drift+dropdown-set / -update(follow-up)--colors/--highlightrevived after canonical schema gainedhighlight_colors/enable_highlight; further loosened to allow shorter color arrays after the schema description was relaxed upstreamFixes per item
#1 workbook-create initial fill — replace
{"sheet_id":""}with{"sheet_name":"Sheet1"}. New workbooks always start with that default sheet;using sheet_name avoids an extra
get_workbook_structureround-trip just tolearn the auto-generated id.
lark_sheet_workbook.go:694-699#2 dropdown data_validation field renames — align body fields with
canonical schema in
sheet-skill-spec/canonical-spec/tool-schemas/tools-schema.json:values→items(option list)multiple_values→support_multiple_values(The
colors/highlight_optionsdeletion that was originally part of thisitem has been undone by #6 below — keep reading.)
Skill ref docs (
lark-sheets-write-cells.md,lark-sheets-batch-update.md)synced.
lark_sheet_write_cells.go#3 dropdown-get range prefix — reuse the existing
splitSheetPrefixedRangehelper to break
Sheet1!C2:C6into sheet_name + bare A1, then thread throughsheetSelectorForToolInput(same pattern as+cells-get).lark_sheet_read_data.go:249-262#4 dim-move endpoint + body key — switch URL from
/dimension_range(v2 insert) to
/move_dimension; rename body keydestinationIndex→destination_indexfor v2 contract consistency.lark_sheet_sheet_structure.go:573,598,621#5 range-sort item validation — walk
--sort-keysclient-side, reject itemsmissing
column(string) orascending(bool) with per-item error messages.Test fixtures and the batch-op contract fixture switched from the historical
{col, order}vocabulary (which the server has never accepted) to the correct{column, ascending}.lark_sheet_range_operations.go:590-625#6 dropdown --colors / --highlight restored, rewired to canonical fields, then length rule loosened
Two-step partial-revert of #2 (commits
8672d8ethen538eb2e):When #2 landed, canonical
set_cell_range.data_validationonly knewhelp_text / items / operator / range / support_multiple_values / type / values, so--colors/--highlighthad no server-side target and wereremoved. Two upstream syncs later changed that:
enable_highlight(bool) +highlight_colors(string[]). Flags brought back, rewired to the new field names.
highlight_colorsdescription loosened — length maynow be shorter than items (server cycles remaining slots through a
built-in 10-color palette). Previously the description required strict
equality.
New mapping:
--colors(string array, hex)highlight_colorscolors--highlight(bool)enable_highlighthighlight_optionsGo-side details across
8672d8e+538eb2e:buildDropdownValidation: re-emits both new fields;--colorslengthcheck is "must not exceed --options length" (loosened from strict
equality to match the new schema).
validateDropdownOptions→validateDropdownOptionsColors: restoredValidate-time check for
+dropdown-update/+dropdown-delete.TestDropdownSet_CellsShapeextended to asserthighlight_colors/enable_highlight(and assert legacycolors/highlight_optionsabsent).
TestDropdownSet_ColorsLongerThanOptions(renamed from_ColorsLengthMismatch): covers the overflow path with 3 colors vs2 options.
TestDropdownSet_ColorsShorterAccepted(new): 2 colors vs 4 optionsis legal and forwarded as-is.
TestDropdownUpdate_BatchPayload: extended to cover propagationthrough
dropdownBatchInput.skills/lark-sheets/references/lark-sheets-*.md,shortcuts/sheets/data/{flag-defs,flag-schemas}.json.The
--options → itemsand--multiple → support_multiple_valuesrenamesfrom #2 are preserved — only the
--colors/--highlightdeletion isundone.
Test plan
go test ./shortcuts/sheets/... -count=1green (full suite + new dropdown coverage)TestDimMove_DryRun,TestExecute_DimMove,TestDropdownSet_CellsShape,TestDropdownSet_ColorsLongerThanOptions,TestDropdownSet_ColorsShorterAccepted,TestDropdownGettable,TestWorkbookCreate_DryRun,TestRangeSort_RejectsMalformedKeys,TestBatchOp_BodyMatchesStandalone/+range-sort,TestDropdownUpdate_BatchPayload)go build ./...greenfeat/lark-sheets-refactor; re-synced from spec MR !7ppe_lark_cli_sheet): replay TC-007 / TC-027030 / TC-046 / TC-038 / R2-003006 / R2-009 / R2-012; plus new dropdown smoke covering--colors/--highlightend-to-end against the upstreamhighlight_colors/enable_highlightfields, with shorter-than-options cases